Conversation
| s3_disable_express_session_auth = config_kwargs[ | ||
| 's3_disable_express_session_auth' | ||
| ] | ||
| preferred_auth_schemes = config_kwargs['auth_scheme_preference'] |
There was a problem hiding this comment.
[Nit, non-blocking]
IMO this variable name should exactly the kwarg name so there's no confusion. There are already a ton of variables around signature version. In this file alone:
signature_version -> sig_version -> requested_auth_scheme, then also you're now adding auth_scheme_preference -> preferred_auth_schemes. It's a lot of complexity to keep in your head, so while it doesn't make a huge difference, I'd prefer to see this renamed to just auth_scheme_preference to make it clear that it's the same thing as the config variable.
|
|
||
| # if a preferred auth schemes list is provided, reorder the auth schemes | ||
| # list based on the preferred ordering. | ||
| if self._preferred_auth_schemes is not None: |
There was a problem hiding this comment.
[Change requested]
We already have a function called resolve_auth_scheme_preference which handles this and should avoid duplicating preference-resolution / ordering logic here.
Right now we manually reorder the endpoint ruleset authSchemes list based on auth_scheme_preference. Instead, we can reuse resolve_auth_scheme_preference() to pick the desired auth type and then map that back to the actual ruleset scheme dict. Here's the proof of concept I wrote:
if self._requested_auth_scheme is not None:
....
elif self._preferred_auth_schemes is not None:
prefs = self._preferred_auth_schemes.split(',')
available_ruleset_names = [s['name'].split('#')[-1] for s in auth_schemes]
auth_schemes_by_auth_type = {self._strip_sig_prefix(s['name'].split('#')[-1]): s for s in auth_schemes}
resolved_auth_type = resolve_auth_scheme_preference(prefs, available_ruleset_names)
name = resolved_auth_type
scheme = auth_schemes_by_auth_type[resolved_auth_type]
We could definitely refactor this and/or the resolve_auth_scheme_preference to make it much cleaner. What do you think?
| @@ -0,0 +1,5 @@ | |||
| { | |||
There was a problem hiding this comment.
High level note:
Jonathan has a semi-related bugfix that will likely need to apply here as well - https://github.com/boto/botocore/pull/3663/changes.
When we resolve v4a via auth scheme preference, we need to make sure we are also respecting the signing region set
2 participants
Notes:
auth_scheme_preferenceonly works as intended for modeled auth schemes at the service level (e.g.bedrock). It does not work for auth being resolved from other points in the precedence chain (e.g. EP2.0, operations).auth_scheme_preferenceto choosesigv4oversigv4a. Outposts models auth at the EP2.0 level, currently.Description of changes:
auth_scheme_preferenceis now used to reprioritize auth schemes, if configured, if the service models auth schemes at the endpoint level.auth_scheme_preferenceis now used to reprioritize auth schemes, if configured, if the service models auth schemes at the operation level.Description of tests:
auth_scheme_preferenceshared config file using the following test cases:Endpoint-level auth test
User configures
sigv4oversigv4a. We expectsigv4to be used.Result:
AWS4-HMAC-SHA256in theAuthorizationheader value confirmssigv4was used.User configures
sigv4aoversigv4. We expectsigv4ato be used.Result:
AWS4-ECDSA-P256-SHA256in theAuthorizationheader value confirmssigv4awas used.By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.